Skip to content

HDFS-16494.Removed reuse of AvailableSpaceVolumeChoosingPolicy#initLocks().#4048

Closed
jianghuazhu wants to merge 1 commit intoapache:trunkfrom
jianghuazhu:HDFS-16494
Closed

HDFS-16494.Removed reuse of AvailableSpaceVolumeChoosingPolicy#initLocks().#4048
jianghuazhu wants to merge 1 commit intoapache:trunkfrom
jianghuazhu:HDFS-16494

Conversation

@jianghuazhu
Copy link
Contributor

@jianghuazhu jianghuazhu commented Mar 4, 2022

Description of PR

When using the default constructor to build the AvailableSpaceVolumeChoosingPolicy, initLocks() is used twice, which is actually unnecessary, the purpose of this pr is to avoid this from happening.
Details: HDFS-16494

How was this patch tested?

Not too stressful for testing. Because this class is relatively mature.

@hadoop-yetus
Copy link

hadoop-yetus commented Mar 4, 2022

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 17m 55s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 💚 mvninstall 34m 48s trunk passed
+1 💚 compile 1m 29s trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚 compile 1m 21s trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 0m 58s trunk passed
+1 💚 mvnsite 1m 29s trunk passed
+1 💚 javadoc 1m 3s trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚 javadoc 1m 27s trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 3m 25s trunk passed
+1 💚 shadedclient 25m 38s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 1m 17s the patch passed
+1 💚 compile 1m 25s the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚 javac 1m 25s the patch passed
+1 💚 compile 1m 13s the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 javac 1m 13s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 53s the patch passed
+1 💚 mvnsite 1m 19s the patch passed
+1 💚 javadoc 0m 53s the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚 javadoc 1m 25s the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 3m 28s the patch passed
+1 💚 shadedclient 25m 29s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 325m 43s /patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt hadoop-hdfs in the patch passed.
+1 💚 asflicense 0m 37s The patch does not generate ASF License warnings.
450m 30s
Reason Tests
Failed junit tests hadoop.hdfs.server.mover.TestMover
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4048/2/artifact/out/Dockerfile
GITHUB PR #4048
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname Linux e83832001c7d 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 57207da
Default Java Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4048/2/testReport/
Max. process+thread count 2343 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4048/2/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

@jianghuazhu
Copy link
Contributor Author

jianghuazhu commented Mar 5, 2022

Here are some unit tests that are failing. E.g:
org.apache.hadoop.hdfs.server.namenode.TestFileTruncate
org.apache.hadoop.hdfs.server.mover.TestMover

It looks like these exceptions have little to do with the update I submitted.
Can you help review this pr, @aajisaka @virajjasani @tomscut .
Thank you very much.

tomscut
tomscut approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@tomscut tomscut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jianghuazhu
Copy link
Contributor Author

jianghuazhu commented Mar 9, 2022

Thanks @tomscut .

@jianghuazhu
Copy link
Contributor Author

jianghuazhu commented Mar 9, 2022

Can you help review this pr, @Hexiaoqiao @ferhui .
Thank you very much.

Hexiaoqiao
Hexiaoqiao approved these changes Mar 12, 2022
View reviewed changes
Copy link
Contributor

@Hexiaoqiao Hexiaoqiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. +1.

@jianghuazhu
Copy link
Contributor Author

jianghuazhu commented Mar 13, 2022

Thanks @Hexiaoqiao .

asfgit pushed a commit that referenced this pull request Mar 16, 2022
@Hexiaoqiao
HDFS-16494.Removed reuse of AvailableSpaceVolumeChoosingPolicy#initLo…
a237526 
…cks(). (#4048). Contributed by JiangHua Zhu.
@Hexiaoqiao Hexiaoqiao closed this Mar 16, 2022
@Hexiaoqiao
Copy link
Contributor

Hexiaoqiao commented Mar 16, 2022

Committed to trunk. Thanks @jianghuazhu for your contributions! Thanks @tomscut reviews!

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022
@Hexiaoqiao
HDFS-16494.Removed reuse of AvailableSpaceVolumeChoosingPolicy#initLo…
bc470d6 
…cks(). (apache#4048). Contributed by JiangHua Zhu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hexiaoqiao Hexiaoqiao Hexiaoqiao approved these changes

@tomscut tomscut tomscut approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jianghuazhu @hadoop-yetus @Hexiaoqiao @tomscut